r.stats: Use long total_count to avoid int overflow #3203
Conversation
HuidaeCho
added
raster
HuidaeCho
changed the title
|
Pylint is broken due to an update of Pylint plugin for pytest. While the Pylint is set to 2.12.2. The pytest plugin updated from 0.19.0 to 0.21.0. That's something which needs to be fixed separately by setting the plugin version to 0.19.0 (fast) or updating to 0.21.0 and fixing the reported issues (slow). Additional checks is broken on main as well due to #3170. |
What about the pytest version?
|
I don't recall. It should be in some of the old runs. Either it was the same (it did not update) or I just thought the plugin is to blame given that it is related to what the plugin does. However, specifying the version of pytest will be needed eventually anyway. |
| @@ -27,7 +27,8 @@ struct Node { | |||
|
|
|||
| static struct Node **hashtable; | |||
| static struct Node *node_list = NULL; | |||
| static int node_count = 0, total_count = 0; | |||
| static int node_count = 0; | |||
| static long total_count = 0; | |||
There was a problem hiding this comment.
Sometimes (Windows?), int might be just as large as long. While the minimums are 16 and 32, sizes can be 32 and 32 or 32 and 64 according to https://en.cppreference.com/w/cpp/language/types (C++, C does not have that nice description but since it depends on the compiler anyway, I guess it applies the same way).
I think someone brought this up in the past, but I don't recall if we have a best practice for that.
There was a problem hiding this comment.
If total_count is guaranteed to be >=0 unsigned int could be a better alternative, otherwise grass_int64 may be considered.
There was a problem hiding this comment.
I chose long because Node.count is long.
There was a problem hiding this comment.
In r.in.pdal, I used typedef which goes for unsigned long or unsigned long long for number of points depending on HAVE_LONG_LONG_INT. However, I think grass_int64 is better as it is aligned with the new(ish) trend of fixed with integers (C99, C++11). Signed versus unsigned and int64_t versus grass_int64, that's whole another discussion.
However, if int->long helps in your case and it fits with the other numbers there, I'm not against merging this as is.
Co-authored-by: Markus Neteler <neteler@gmail.com>
This PR fixes an integer overflow (sum % with
-phigher than 100%) by usinglong total_count.